Propose a replace visit on redirection
#328
Conversation
…nd location * Updated the browser adapter to remove the call to `followRedirect` as it now happens in the visit itself * Updated the tests to reflect the new behaviour
| @@ -24,7 +24,7 @@ export class FormSubmissionTests extends TurboDriveTestCase { | |||
|
|
|||
| this.assert.notOk(await this.formSubmitted) | |||
| this.assert.equal(await this.pathname, "/src/tests/fixtures/one.html") | |||
| this.assert.equal(await this.visitAction, "advance") | |||
| this.assert.equal(await this.visitAction, "replace") | |||
There was a problem hiding this comment.
Is replace the expected action for GET form submissions?
It looks like we'll want a new test to validate normal visit redirects result in a new visit proposal with a replace action.
There was a problem hiding this comment.
The sequence:
- Visit the original URL with advance action
- If there is a redirection, propose replace visit to the destination with the existing response. That way it doesn't fire one more fetch call, instead reuses the existing response.
User will see the same behaviour as before. Just that programmatically the sequence looks like advance + replace instead of just advance.
As long as there is a redirect, replace action is expected at the end. In this test, form is submitted to /__turbo/redirect (ref: https://github.com/jaysson/turbo/blob/replace-on-redirect/src/tests/fixtures/form.html#L41), and that endpoint returns a 303 to /src/tests/fixtures/one.html resulting in replace action.
It looks like we'll want a new test to validate normal visit redirects result in a new visit proposal with a replace action.
Yes, will do.
|
@jayohms Added two tests. One verifies that the replace visit is happening, and another verifies that it's not replacing the current page, but pushes a new one in the navigation stack. |
|
Thanks @jaysson, I'll take a closer look at this before the end of the week and test in the native apps. |
|
Fixed the merge and tests passed 👍 |
|
@jayohms @jaysson might there be an alternative way to achieve this without proposing another Visit? I work on an animation library that animates on Related: #877 |
|
@domchristie The main issue I see is that we map url paths to view controllers/fragments in the path configuration files. The path properties that each path maps to in the path config files determine how/where to navigate. The turbo native libraries propose visits and load the view controller/fragment before the visit actually starts. So:
Since the original url can have different path properties than the redirect url, the double-visit is the best way to work around these issues. Maybe passing along metadata with the |
followRedirectas it now happens in the visit itselfThis should also provide a solution to these issues on native adapters:
In addition, it allows path properties to be applied correctly to redirection destinations in native adapters.